[lua] Expeditionary Force Battle Logic#10510
Conversation
fb4484e to
3e945c0
Compare
| ----------------------------------- | ||
| -- Runtime state. One record per EF zone, built lazily on zone initialize. | ||
| -- If you modify this lua file during runtime, you must relaunch map as expForceZoneData is erased. | ||
| local expForceZoneData = {} |
There was a problem hiding this comment.
ehhh, no, lets not do that if we dont need to, if this is a table that will contain info, place the information here.
There was a problem hiding this comment.
I was filling out this table like
[xi.zone.BEAUCEDINE_GLACIER] = { state = bannerState.IDLE, nms = {}, gone = {}, numAlive = 0, bannerIndex = nil },
for each entry, when I noticed I can rewrite the code to get rid of gone and numAlive, and change nms to nmsAlive = {}. I'll just remove the IDs from nmsAlive as they die and track that way. I didn't want to rewrite while you were reviewing though.
There was a problem hiding this comment.
My point is this isnt needed. Combine it with what I said bellow.
The ONLY thing we need to track which EF is active per-zone is a local variable in the banner npc.
We dont need to build a dynamic table, we already have the data.
When we pop the 4 nms, or whatever number we need per banner, we only need to check in their onMobDespawn if theres any other nm in their set currently still spawned, and if not, the EF is completed.
There was a problem hiding this comment.
That makes sense. Updated and tested the code.
| local cap = zoneInfoTable[zoneId].levelCap | ||
| if cap == 99 then | ||
| cap = xi.settings.main.MAX_LEVEL | ||
| end |
There was a problem hiding this comment.
Why dont you just define the cap as this directly?
| end) | ||
| end | ||
|
|
||
| -- Safety check every 60s while banner is active. Catches the case where a DESPAWN listener misses. |
There was a problem hiding this comment.
Why would this happen though?
There was a problem hiding this comment.
If an error occurred during the death or despawn code. But, thinking about it, that would mask a real bug.
I also don't know exactly how zone cleanup occurs and was worried zone cleanup would skip despawn code. I'm fine if we get rid of it.
There was a problem hiding this comment.
Removed the watchDog.
| -- Catch case when pool has fewer than 4 mobs | ||
| if mobId == nil then | ||
| break | ||
| end |
There was a problem hiding this comment.
Several things.
For starters, insted of pulling by mob pool, why dont you define a table with actual mobid entries in it? It would help visuallize AND it wouldnt be subject to getting their pools changed in the future and going unotized.
Secondly, with that, you could just go through the entires 1 by 1 in the loop without needing to have this fail-safe, via ipairs.
There was a problem hiding this comment.
For Buburimu as an example, I could do
local nmPoolTable =
{
[xi.zone.BUBURIMU_PENINSULA] =
{
[xi.mobFamily.GOBLIN] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BEASTMASTER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_DARK_KNIGHT,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RANGER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RED_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_THIEF,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WARRIOR,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WHITE_MAGE,
},
[xi.mobFamily.YAGUDO] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BARD,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_MONK,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_NINJA,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SAMURAI,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SUMMONER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_WHITE_MAGE,
}, ...
and then
```local candidates = utils.shuffle(nmPoolTable[zoneId][bannerInfo.mobFamily])```
The IDs aren't always sequential because of pets.
Is this what you had in mind?
Do you want me to add all the mobs to the 13 zone IDs.lua files in this PR? Or the follow up PR?
There was a problem hiding this comment.
keying it by mob pool would defeat the purpose of my comment.
My idea of a better organization, would be like this:
First: a table with the zones with the banners, like so...
local zoneBanner =
{
[xi.zone.BUBURIMU_PENINSULA] =
{
[1] = { position = { 101.491, -23.090, 199.798, 218 } },
[2] = { position = { 527.885, 0.486, -40.241, 157 } },
[3] = { position = { 315.895, -0.025, 361.453, 17 } },
[4] = { position = { -132.589, 20.000, -314.261, 230 } },
[5] = { position = { -446.510, -8.799, -282.799, 240 } },
}
}The keys are important. Each banner would have a key associated, meaning, we can set a local variable to the banner with the key, which we can fetch at any time, and use it as a master key for everything else.
Then, a nmTable for nms, keyed with the banners same key
local bannerNMs =
{
[xi.zone.BUBURIMU_PENINSULA] =
{
[1] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BEASTMASTER, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_DARK_KNIGHT, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RANGER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RED_MAGE, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_THIEF,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WARRIOR, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WHITE_MAGE,
},
[2] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BEASTMASTER, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_DARK_KNIGHT, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RANGER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_RED_MAGE, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_THIEF,
zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WARRIOR, zones[xi.zone.BUBURIMU_PENINSULA].mob.HOBGOBLIN_WHITE_MAGE,
},
[3] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BARD, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_MONK, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_NINJA,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SAMURAI, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SUMMONER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_WHITE_MAGE,
},
[4] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BARD, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_MONK, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_NINJA,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SAMURAI, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SUMMONER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_WHITE_MAGE,
},
[5] =
{
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BARD, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_BLACK_MAGE,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_MONK, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_NINJA,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SAMURAI, zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_SUMMONER,
zones[xi.zone.BUBURIMU_PENINSULA].mob.THEOYAGUDO_WHITE_MAGE,
},
},
}This defines very explicitly what the NMs can be per banner, and is safe from both shifts in IDs and in mob pools.
It would be a big list, sure. But we wouldnt need to build a table anywhere, which is abstract AF, we wouldnt need to restart the server to test when editing the file and more importantly, its more clear for humans, which are the actual end users. The more crystal clear we make this, the more ppl can jump in and debug in the case they find an issue. And by extension, the more chances we will actually get a PR with said hypothetical fix.
There was a problem hiding this comment.
Added. I also added the 13 IDs.lua files.
I did two variables on the banner (State and BannerIndex).
| local pos = GetFurthestValidPosition(banner, distance, angle) -- Drops mob on valid ground and snaps closer if terrain blocks the distance. | ||
|
|
||
| local mob = GetMobByID(mobId) | ||
| if mob == nil then |
There was a problem hiding this comment.
in general, you dont need to check for == nil or ~= nil
lets say mob
if mob then
if not mob then
| if newBannerIndex == lastBannerIndex then | ||
| newBannerIndex = #bannerOptions | ||
| end | ||
| end |
There was a problem hiding this comment.
we could instead do this
local availableOptions = {}
for i = 1, ##bannerOptions do
if i ~= lastBannerIndex then
table.insert(availableOptions , #bannerOptions + 1, i)
end
end
newBannerIndex = math.randomInt(1, #availableOptions)This guarantees a different position. Of course, that would be if you had the tables keyed.
There was a problem hiding this comment.
I did
local availableOptions = {}
for i = 1, #bannerOptions do
if i ~= lastBannerIndex then
table.insert(availableOptions, i)
end
end
local newBannerIndex = availableOptions[math.randomInt(1, #availableOptions)]
f34e3b9 to
c5c7726
Compare
| HOBGOBLIN_RED_MAGE = GetFirstID('Hobgoblin_Red_Mage'), | ||
| HOBGOBLIN_THIEF = GetFirstID('Hobgoblin_Thief'), | ||
| HOBGOBLIN_WARRIOR = GetFirstID('Hobgoblin_Warrior'), | ||
| HOBGOBLIN_WHITE_MAGE = GetFirstID('Hobgoblin_White_Mage'), |
There was a problem hiding this comment.
In general, applied to the whole PR: mobs and npcs should be in alphabetical order. Easier to read or find a concrete mob
There was a problem hiding this comment.
Changed. And updated alphabetical order for those files except for HARVESTING and LOGGING which in their own space below and OPTION_ONE, OPTION_TWO, OPTION_THREE.
| -- Early Return: Only fires once for the killer | ||
| if not optParams.isKiller then | ||
| return | ||
| end |
There was a problem hiding this comment.
if you kill with a DoT, there wont ever be a killer. can check that with optParams.noKiller
There was a problem hiding this comment.
Added the check. Tested on local. DoT killer will check for all mobs dead but not reward anything.
| for _, nmId in ipairs(bannerNMs[zoneId][bannerIndex]) do | ||
| if nmId ~= mobId and GetMobByID(nmId):isAlive() then | ||
| nmsRemain = true | ||
| break |
There was a problem hiding this comment.
dont need nmsRemain variable
for _, nmId in ipairs(bannerNMs[zoneId][bannerIndex]) do
if nmId ~= mobId and GetMobByID(nmId):isAlive() then
return
end
endThere was a problem hiding this comment.
If I make that change in onMobDeath, it will never run the rewards code below, so I did not change it there.
I made this change in onMobDespawn.
In onMobDeath I changed the variable to allDefeated to avoid the double negative later.
| -- SEND ZONE MESSAGE | ||
| for _, person in pairs(mob:getZone():getPlayers()) do | ||
| person:messageText(person, ID.text.EXP_FORCE_KILL_SANDORIA + creditNation, 5) -- 5 = Grey: messageText event | ||
| end |
There was a problem hiding this comment.
Is this actually true? every single person in the zone, no matter where they are, get the message?
There was a problem hiding this comment.
Here's a non-Bastok, non-party member, without the KI, getting the message within close range. https://youtu.be/lBNlgznk_xM?t=600
Here's a Batsok player who left party and ran pretty far away who got the message. https://youtu.be/lBNlgznk_xM?t=1454
I also asked another player in the zone and they acknowledged they saw them. (No idea where they were.)
It appears to be zone wide.
c5c7726 to
8ff9f35
Compare
8ff9f35 to
133baeb
Compare
I affirm:
What does this pull request do?
This PR implements the main battle logic for the Expeditionary Force feature.
This is not being called from anywhere yet. The logic is just being created. A follow up PR for each of the 13 zones will implement mobs, update Zone.lua initialization, and add IDs to IDs.lua.
The NM spawn logic was reviewed and updated during PR #10278
Function Call Examples
Each zone IDs.lua file will need the below.
Here is an example mob file and how it will hook in
Here is an example pet file
Here is an example Beastmens_Banner.lua file
zone.lua will have
xi.expeditionaryForce.initZone(zone)added to it's initialization.Captures
Buburimu Peninsula
https://youtu.be/l9DzBCHJxr8
Capture: https://drive.google.com/file/d/1szyT6BYfNTdI4UBYFExiy_pFMZMwrCIh/view?usp=sharing
Capture of a group of 4 doing Expeditionary Force for Bastok when it was in third place in Buburimu Peninsula. We wiped once and then cleared it another time.
Buburimu Peninsula
Capture: https://drive.google.com/file/d/1aptoq5gOyY7UojP3DpxfVOMR1ceZDr1l/view?usp=drive_link
Video 1: https://youtu.be/B_SVTXtci9w
Video 2: https://youtu.be/vl-E0Vs7r70
Expeditionary Force (Bastok) - Valkurm Dunes + Xarcabard
https://youtu.be/uuk4aSnlES4
Capture: https://drive.google.com/file/d/1zN2l2B8aRyUAg8i1c3KZ2Dwg8C1lT8nX/view?usp=sharing
Expeditionary Force (Bastok) - Valkurm Dunes + Xarcabard from a non-Bastok (for half) player. It mirrors Kipling's capture above.
https://youtu.be/lBNlgznk_xM
Capture: https://drive.google.com/file/d/1PtyPYbbVMKLeLgUShDYWsG91Ur4gYk_6/view?usp=drive_link
Expeditionary Force (Bastok) - Beaucedine Glacier + Cape Terrigan
https://youtu.be/UliVwqkLRNM
Capture: https://drive.google.com/file/d/103ekoyfjW3A7I_GWk39vivPU0LP6XSDF/view?usp=sharing
Expeditionary Force (Bastok) - Beaucedine Glacier + Cape Terrigan (Same as Kipling's above from another PoV)
Capture: https://drive.google.com/file/d/112OqWeZc27rpHqe71SJUDkxF0dnLW8Jl/view?usp=drive_link
https://youtu.be/1cwPNbnQqwY
Expeditionary Force - Banner locations for all zones except Junger, Qufim, and Eastern Altepa
Screenshot Files: https://drive.google.com/file/d/1QOVac-LLcGw8EooHTM-EKHvpBQTrsnPg/view?usp=drive_link
Expeditionary Force - Solo Spawning Flags
Zones: Beaucedine Glacier, Cape Teriggan, Eastern Altepa Desert, Meriphataud Mountains, Pashhow Marshlands, The Sanctuary of Zi'Tah, Valkurm Dunes, Xarcabard, Yhoator Jungle, Yhutunga Jungle
I solo spawned the mobs at the banners, died, and repeated until I discovered all 5 banners for the zone.
Capture: https://drive.google.com/file/d/1wlzjEEqKvCoPNL5fhsla-cTXGSjnALZL/view?usp=drive_link
Expeditionary Force (Bastok) - Zulkheim region - Conquest Influence Test against Coffer
Leveled a character from 7 to 34 in the region, died in the middle. Then I opened a coffer. Leveling had just below a 1% swing. Coffer had about a 2% swing.
https://drive.google.com/file/d/1edFGvLuwAKTZJFRC_3JE5oKnKpj4qRZv/view?usp=drive_link
Expeditionary Force - Junger and Altepa Banner Spawning
Just running to a flag, spawning mobs, and dying.
Capture: https://drive.google.com/file/d/1NbQElIQVQ09KkKn3sG8wqgUdMtnrqM61/view?usp=drive_link
Screenshot Files of Banners: https://drive.google.com/file/d/1Kvs2qMe6Mw4rLAuwlDnVetvbLGVkOKo4/view?usp=drive_link
Steps to test these changes
To test these changes, you need to update a Banner, add the NMs, and update the Mob lua files. See PR #10278 for how to do this. Once you've completed that, give yourself a EF key item and start at step 4 on this test plan: https://docs.google.com/document/d/1OrlcOkArFUCCBeQB5HzSWt3SfA0dn1iK/edit